Skip to content

Conversation

@cchung100m
Copy link
Contributor

@cchung100m cchung100m commented Nov 20, 2025

This PR is trying to fix issues #17798.

Root Cause

The error message construction used it->second unconditionally, but the Verify() condition was:

  • it == currently_defined_.end() || redefine_is_allowed

This means when the condition evaluates to false (triggering the error), it could be due to:

  1. it != end() && !redefine_is_allowed => Safe to access it->second
  2. it == end() && !redefine_is_allowed => Invalid to access it->second

Solution

The fix ensures safe iterator access by:

  1. Storing the Verify result: Instead of chaining error messages directly to Verify(), store the result in a variable
  2. Conditional dereferencing: Only access it->second when it != end()
  3. Meaningful error messages: Provide appropriate messages for both cases

@cchung100m cchung100m force-pushed the issue-17798 branch 2 times, most recently from a5c61b5 to 07b3101 Compare November 21, 2025 00:35
@cchung100m cchung100m force-pushed the issue-17798 branch 3 times, most recently from 5b22c60 to e54214c Compare November 21, 2025 14:43
@cchung100m cchung100m marked this pull request as ready for review November 22, 2025 06:43
Copy link
Member

@tlopex tlopex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you

@tlopex tlopex merged commit 9826150 into apache:main Nov 25, 2025
12 checks passed
@cchung100m cchung100m deleted the issue-17798 branch November 25, 2025 05:43
@cchung100m
Copy link
Contributor Author

Thanks to @tlopex 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants